Fix JDBC parser parameter counting for empty comments and SELECT * EXCEPT - #3067
Fix JDBC parser parameter counting for empty comments and SELECT * EXCEPT#3067n200534 wants to merge 1 commit into
Conversation
|
|
|
@cursor review |
There was a problem hiding this comment.
Pull request overview
This PR fixes JDBC prepared-statement parameter counting regressions in jdbc-v2 by addressing (1) an off-by-one bug in empty single-line comment skipping and (2) a missing SELECT * EXCEPT (...) construct in the bundled ANTLR4 grammar, which previously caused some parser backends to miss later ? placeholders.
Changes:
- Fix
ClickHouseUtils.skipSingleLineComment(...)to correctly advance past a newline when the comment body is empty (--\n...). - Extend the ANTLR4 grammar to parse
* EXCEPT (...)in select lists so parse-tree-based JDBC parsers can continue scanning and find later?markers. - Add regression coverage in both
clickhouse-datautility tests andjdbc-v2parser facade tests, and document the user-visible fix inCHANGELOG.md.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| jdbc-v2/src/test/java/com/clickhouse/jdbc/internal/BaseSqlParserFacadeTest.java | Adds regression SQL cases to ensure arg counting remains correct with empty -- lines and SELECT * EXCEPT (...). |
| jdbc-v2/src/main/antlr4/com/clickhouse/jdbc/internal/parser/antlr4/ClickHouseParser.g4 | Updates the ANTLR grammar to allow * select items to include an optional EXCEPT modifier. |
| clickhouse-data/src/test/java/com/clickhouse/data/ClickHouseUtilsTest.java | Adds a focused unit test for skipping an empty -- comment line. |
| clickhouse-data/src/main/java/com/clickhouse/data/ClickHouseUtils.java | Fixes the newline detection condition in skipSingleLineComment(...) so empty comments don’t terminate scanning early. |
| CHANGELOG.md | Documents the jdbc-v2 prepared-statement parameter counting fix and links issue #3052. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit c1daab8. Configure here.
Summary
--is followed immediately by a newline, so later JDBC parameter markers are still discoveredSELECT * EXCEPT (...), including identifier-list and regular-expression formsSELECT * EXCEPT (...)across all three SQL parser backendsCHANGELOG.mdCloses #3052
Root cause
ClickHouseUtils.skipSingleLineCommenttreated a newline located exactly at its start index as if no newline existed, causing the parameter scanner to skip the rest of the SQL. Separately, the bundled ANTLR4 grammar accepted*in a select list but did not model ClickHouse'sEXCEPTmodifier, so the parse-tree-based backend never reached a later?marker.User impact and compatibility
Prepared statements containing either construct now report the correct parameter count and can bind normally instead of failing with
ArrayIndexOutOfBoundsException. This does not change public APIs, configuration, wire protocols, or binary compatibility.Validation
mvn -pl jdbc-v2 -am -DskipTests install— passed under JDK 17mvn -pl jdbc-v2 test— 1,331 tests passedmvn -pl clickhouse-data test— 1,554 tests passed, 113 conditionally skipped, 0 failures/errorsChecklist
--comment;SELECT * EXCEPT (...)), causing ArrayIndexOutOfBoundsException on bind #3052